Skip to content

Add check for Reviewed-By: line to merge.sh#64363

Open
sxa wants to merge 1 commit into
nodejs:mainfrom
sxa:merge_reviewer_check
Open

Add check for Reviewed-By: line to merge.sh#64363
sxa wants to merge 1 commit into
nodejs:mainfrom
sxa:merge_reviewer_check

Conversation

@sxa

@sxa sxa commented Jul 8, 2026

Copy link
Copy Markdown
Member

Identified after I used merge.sh myself and it didn't pick up the absence of the Reviewed-By: lines
This will stop it happening again.

Signed-off-by: Stewart X Addison <sxa@ibm.com>
@sxa sxa requested a review from aduh95 July 8, 2026 15:20
@sxa sxa self-assigned this Jul 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jul 8, 2026
Comment thread tools/actions/merge.sh
Comment on lines +59 to +63
git log -1 HEAD --pretty='format:%B' | git interpret-trailers --parse --no-divider | \
grep -q "^Reviewed-By: " || {
echo "No Reviewed-By: lines in the commit message"
exit 1
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really it should run core-validate-commit, there are many more things that could go wrong – maybe we can also document somewhere that humans are never ever supposed to write a PR-URL trailer manually

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sufficiently familiar with core-validate-commit to make that change but if it can be called from here then that would probably make more sense.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also considered adding an extra message to the error here (and on the PR-URL check) to say "Have you run 'git node land' first?" which would discourage an manual adding of the lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants